fix space id extraction #316
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
nikgraf
force-pushed
the
ng/fix-space-id-usage
branch
from
31be751 to
ff51bd0
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR ensures the space parameter is consistently captured and forwarded through query hooks, adds an explicit fallback for missing context, and updates the Playground component to accept and propagate a spaceId.
- Propagate the
spaceargument fromuseQueryintouseQueryPublicanduseQueryLocal - Introduce
spaceinQueryPublicParamsandQueryParams - Provide a default fallback in
useHypergraphSpaceInternaland update Playground to takespaceId
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/hypergraph-react/src/use-query.tsx | Forward space to useQueryPublic |
| packages/hypergraph-react/src/internal/use-query-public.tsx | Destructure/merge space from params and context |
| packages/hypergraph-react/src/internal/types.ts | Add space to QueryPublicParams |
| packages/hypergraph-react/src/HypergraphSpaceContext.tsx | Return default { space: '' } if context is missing |
| apps/events/src/routes/playground.lazy.tsx | Pass spaceId into Playground and wrap children in provider |
| apps/events/src/components/playground.tsx | Change Playground signature to accept spaceId |
Comments suppressed due to low confidence (2)
packages/hypergraph-react/src/internal/types.ts:6
- [nitpick] Consider renaming this
spaceproperty tospaceIdto align with other APIs and clarify that it represents an identifier.
space?: string | undefined;
packages/hypergraph-react/src/use-query.tsx:149
- Add unit tests to verify that the
spaceparameter is correctly forwarded touseQueryPublicand that context fallback behaves as expected.
const publicResult = useQueryPublic(type, { enabled: mode === 'public', include, first, space });
| export function useHypergraphSpaceInternal() { | ||
| const context = useContext(HypergraphReactContext); | ||
| return context as HypergraphContext; | ||
| return (context as HypergraphContext) || { space: '' }; |
There was a problem hiding this comment.
Fallbacking to an empty string when context is missing can mask missing provider issues. Consider throwing an error or logging a warning to catch unintended usage early.
Suggested change
| return (context as HypergraphContext) || { space: '' }; | |
| if (!context) { | |
| throw new Error('HypergraphReactContext provider is missing. Ensure that HypergraphSpaceProvider is used.'); | |
| } | |
| return context as HypergraphContext; |
| // TODO: for multi-level nesting it should only allow the allowed properties instead of Record<string, Record<string, never>> | ||
| include?: { [K in keyof Schema.Schema.Type<S>]?: Record<string, Record<string, never>> } | undefined; | ||
| space?: string; | ||
| space?: string | undefined; |
There was a problem hiding this comment.
[nitpick] The explicit | undefined is redundant for an optional property; you can simplify this to space?: string for clarity.
Suggested change
| space?: string | undefined; | |
| space?: string; |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.